Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent gcc -Werror=maybe-uninitialized warnings in spa_wait_common() #9326

Merged

Conversation

loli10K
Copy link
Contributor

@loli10K loli10K commented Sep 15, 2019

Motivation and Context

Build is failing on my Debian9 builder:

     CC [M]  module/zfs/spa.o
   module/zfs/spa.c: In function ‘spa_wait_common.part.31’:
   module/zfs/spa.c:9468:6: error: ‘in_progress’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
      if (!in_progress || spa->spa_waiters_cancel || error)
         ^
   cc1: all warnings being treated as errors

Description

in_progress may be used uninitialized if we return EINVAL in spa_vdev_initializing():

spa_wait_common()
  -> spa_activity_in_progress()
    -> spa_vdev_initializing() <- returns EINVAL, does not initialize "in_progress"

When this happens error is guaranteed to be != 0; this change prevents access to the uninitialized in_progress thanks to short-circuit evaluation

diff --git a/module/zfs/spa.c b/module/zfs/spa.c
index 0f1a2a9eb..7e5c474eb 100644
--- a/module/zfs/spa.c
+++ b/module/zfs/spa.c
@@ -9465,7 +9465,7 @@ spa_wait_common(const char *pool, zpool_wait_activity_t activity,
                error = spa_activity_in_progress(spa, activity, use_tag, tag,
                    &in_progress);
 
-               if (!in_progress || spa->spa_waiters_cancel || error)
+               if (error || !in_progress || spa->spa_waiters_cancel)
                        break;
 
                *waited = B_TRUE;

How Has This Been Tested?

Compiled on Debian9 builder (GCC 6.3.0)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (a change to man pages or other documentation)

Checklist:

This commit fixes the following build failure detected on Debian9
(GCC 6.3.0):

     CC [M]  module/zfs/spa.o
   module/zfs/spa.c: In function ‘spa_wait_common.part.31’:
   module/zfs/spa.c:9468:6: error: ‘in_progress’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
      if (!in_progress || spa->spa_waiters_cancel || error)
         ^
   cc1: all warnings being treated as errors

Signed-off-by: loli10K <[email protected]>
@loli10K loli10K added the Status: Code Review Needed Ready for review and testing label Sep 15, 2019
Copy link
Contributor

@chrisrd chrisrd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Sep 16, 2019
@behlendorf behlendorf merged commit b24771a into openzfs:master Sep 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants